Skip to content

[improve][broker] Replace ScheduledExecutorService to ExecutorService in ModularLoadManagerImpl - #19656

Merged
AnonHxy merged 1 commit into
apache:masterfrom
AnonHxy:opt_loadManager_executors
Mar 1, 2023
Merged

[improve][broker] Replace ScheduledExecutorService to ExecutorService in ModularLoadManagerImpl#19656
AnonHxy merged 1 commit into
apache:masterfrom
AnonHxy:opt_loadManager_executors

Conversation

@AnonHxy

@AnonHxy AnonHxy commented Feb 28, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Motivation

An minor improvement by replacing ScheduledExecutorService to ExecutorService in ModularLoadManagerImpl

Modifications

Replace ScheduledExecutorService to ExecutorService in ModularLoadManagerImpl

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Verifying this change

  • Make sure that the change passes the CI checks.

Matching PR in forked repository

AnonHxy#30

@github-actions

github-actions Bot commented Feb 28, 2023 •
edited by AnonHxy
Loading

Copy link
Copy Markdown

@AnonHxy Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

@AnonHxy AnonHxy mentioned this pull request Feb 28, 2023
Closed
@github-actions github-actions Bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Feb 28, 2023
@AnonHxy AnonHxy self-assigned this Feb 28, 2023
@315157973

315157973 commented Feb 28, 2023

Copy link
Copy Markdown
Contributor

/pulsarbot run-failure-checks

@AnonHxy AnonHxy added this to the 3.0.0 milestone Mar 1, 2023
@AnonHxy
AnonHxy merged commit 145e985 into apache:master Mar 1, 2023
@heesung-sohn

heesung-sohn commented Mar 4, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@AnonHxy

The PR is causing a test failure.

 Check failure on line 785 in pulsar-broker/src/test/java/org/apache/pulsar/broker/namespace/NamespaceServiceTest.java

GitHub Actions
/ Flaky tests suite
NamespaceServiceTest.testModularLoadManagerRemoveBundleAndLoad
Cannot locate field scheduler on class org.apache.pulsar.broker.loadbalance.impl.ModularLoadManagerImpl

#17487 (comment)

@heesung-sohn heesung-sohn mentioned this pull request Mar 4, 2023
Merged
4 tasks
@AnonHxy

AnonHxy commented Mar 4, 2023

Copy link
Copy Markdown
Contributor Author

@AnonHxy

The PR is causing a test failure.

Thanks for your reminder. Fix it in this PR #19710 . PTAL

@AnonHxy AnonHxy mentioned this pull request Mar 4, 2023
Merged
5 tasks
shibd pushed a commit to shibd/pulsar that referenced this pull request Oct 24, 2023
@AnonHxy @shibd
[improve][broker] Replace ScheduledExecutorService to ExecutorService…
f50f126 
… in ModularLoadManagerImpl (apache#19656)

(cherry picked from commit 145e985)
shibd pushed a commit to shibd/pulsar that referenced this pull request Oct 24, 2023
@AnonHxy @shibd
[improve][broker] Replace ScheduledExecutorService to ExecutorService…
2f5e753 
… in ModularLoadManagerImpl (apache#19656)

(cherry picked from commit 145e985)
@shibd shibd mentioned this pull request Oct 24, 2023
Closed
14 tasks
@lhotari

lhotari commented Mar 5, 2024

Copy link
Copy Markdown
Member

Why was this cherry-picked to branch-2.11? The tests are broken in branch-2.11

  Error:  Tests run: 42, Failures: 1, Errors: 0, Skipped: 36, Time elapsed: 21.939 s <<< FAILURE! - in org.apache.pulsar.broker.namespace.NamespaceServiceTest
  Error:  testModularLoadManagerRemoveBundleAndLoad(org.apache.pulsar.broker.namespace.NamespaceServiceTest)  Time elapsed: 2.615 s  <<< FAILURE!
  org.powermock.reflect.exceptions.FieldNotFoundException: No instance field named "scheduler" could be found in the class hierarchy of org.apache.pulsar.broker.loadbalance.impl.ModularLoadManagerImpl.
  	at org.powermock.reflect.internal.matcherstrategies.FieldNameMatcherStrategy.notFound(FieldNameMatcherStrategy.java:40)
  	at org.powermock.reflect.internal.WhiteboxImpl.findSingleFieldUsingStrategy(WhiteboxImpl.java:502)
  	at org.powermock.reflect.internal.WhiteboxImpl.findFieldInHierarchy(WhiteboxImpl.java:455)
  	at org.powermock.reflect.internal.WhiteboxImpl.findFieldInHierarchy(WhiteboxImpl.java:443)
  	at org.powermock.reflect.internal.WhiteboxImpl.getInternalState(WhiteboxImpl.java:427)
  	at org.powermock.reflect.Whitebox.getInternalState(Whitebox.java:292)
  	at org.apache.pulsar.broker.namespace.NamespaceServiceTest.waitResourceDataUpdateToZK(NamespaceServiceTest.java:830)
  	at org.apache.pulsar.broker.namespace.NamespaceServiceTest.testModularLoadManagerRemoveBundleAndLoad(NamespaceServiceTest.java:789)
  	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
  	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
  	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
  	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
  	at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:45)
  	at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:73)
  	at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:11)
  	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
  	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
  	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
  	at java.base/java.lang.Thread.run(Thread.java:840)

@lhotari

lhotari commented Mar 5, 2024

Copy link
Copy Markdown
Member

Nevermind, cherry-picking and backporting #19710 to fix the issue in branch-2.11.

nodece pushed a commit to nodece/pulsar that referenced this pull request Jul 23, 2024
@AnonHxy @nodece
[improve][broker] Replace ScheduledExecutorService to ExecutorService…
8ec75c2 
… in ModularLoadManagerImpl (apache#19656)

(cherry picked from commit 145e985)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
nodece pushed a commit to ascentstream/pulsar that referenced this pull request Jul 24, 2024
@AnonHxy @nodece
[improve][broker] Replace ScheduledExecutorService to ExecutorService…
d179a6b 
… in ModularLoadManagerImpl (apache#19656)

(cherry picked from commit 145e985)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@315157973 315157973 315157973 approved these changes
@coderzc coderzc coderzc approved these changes
@merlimat merlimat Awaiting requested review from merlimat
@codelipenghui codelipenghui Awaiting requested review from codelipenghui

Assignees

@AnonHxy AnonHxy

Labels

cherry-picked/branch-2.11 doc-not-needed Your PR changes do not impact docs ready-to-test release/2.11.3

Projects

None yet

Milestone

3.0.0

Development

Successfully merging this pull request may close these issues.

6 participants

@AnonHxy @315157973 @heesung-sohn @lhotari @coderzc @shibd